Repository navigation
fix(renovate): move concurrency to job level - #35
Conversation
A workflow level concurrency group queues every triggering event before any job's if runs. One Renovate run rebasing several pull requests fires a burst of pull_request edited events, all of them ones the anti-loop guard would have skipped. They pile into that queue, get cancelled while merely pending, and report as a failed check on each rebased pull request. The guard is correct and never executes. That is the whole defect, and the opposite of what the symptom suggests. At job level the if runs first, so a run that was always going to be skipped is reported skipped rather than cancelled. A per pull request group was rejected: Renovate processes the whole repository on every run, so two runs admitted to different groups are two concurrent Renovate processes against one repository, which the guard does nothing to prevent. Dropping the pull_request trigger was rejected because it costs the rebase checkbox. Signed-off-by: Simon Ostendorf <github@simon-ostendorf.de>
|
Verdict: approve Judgement on the evidence questionThe ordering claim is strong enough to ship, though not from a verbatim GitHub sentence, exactly as the PR says. Verified independently:
Net: two of the three cited sources check out as described; the third (the community report) is plausible but not independently confirmed. The change is also safe on the downside: it cannot make a genuinely-triggered run behave differently (same group key, same FindingsBlocker: none. Should: none. Nit:
Other checks
Verification run locally (worktree, head
|
|
Correcting my own evidence claim in the description, and recording the argument that actually settles this. One of the three cited sources does not hold up. The review could not locate the GitHub community report of this exact cancellation pattern; adjacent discussions support parts of the mechanism but not a match. I should not have cited it as corroboration without reproducing it. The remaining two stand: GitHub's documented context asymmetry for job-level concurrency, and the Gitea pull request written explicitly to match GitHub's semantics, which is real and says what I said it says. The stronger argument is one I did not make, and it is the review's: this change is asymmetric-safe. The group key, the The real test remains the first Renovate burst after merge: runs that would previously have been cancelled should report Noted the nit on |
The finding, stated first because the symptom suggests the opposite
The anti-loop guard is correct and never executes. It is not broken and nothing about it changes here.
A workflow-level concurrency group is resolved when the event arrives, before any job's
if:runs. So a run the guard was always going to skip gets queued anyway, and can then be cancelled while still pending. A cancelled run reports as a failed check on whatever pull request triggered it.What was observed
One Renovate run rebasing several pull requests fires a burst of
pull_request: editedevents, every one of them sender-filtered by the guard. In a caller repository:Six Renovate pull requests were left with a red check that had nothing to do with their contents. The two
skippedruns at the end are the guard working normally, which is what makes the cancelled ones look like a different problem.The change
concurrency:moves from workflow level to job level, insidejobs.renovate. Same group, samecancel-in-progress: false. Theif:is untouched.At job level the
if:is evaluated first, so a job that skips never joins the group and there is nothing to cancel.On the evidence for that ordering
Worth being precise, because the whole fix rests on it.
GitHub's documentation does not state outright that a job's
if:is evaluated before it joins its own concurrency group. What it does state is thatjobs.<job_id>.concurrencymay use theneeds,strategyandmatrixcontexts, which workflow-level concurrency cannot. That asymmetry only makes sense if job-level concurrency is resolved when the job is about to start, after its dependency graph and condition are known.The behavioural claim is corroborated by two independent sources rather than inferred from that alone: a GitHub community report describing exactly this cancellation pattern and its resolution, and a Gitea pull request titled "evaluate job-level
if:before concurrency check", written specifically to match GitHub's real semantics, whose summary notes that skipped jobs no longer occupy concurrency slots.So: confirmed by cross-implementation evidence, not by a verbatim documentation sentence. Flagging that rather than overstating it.
Alternatives rejected
A per-pull-request group. It stops the pile-up, but Renovate processes the whole repository on every run, not just the triggering pull request. Two runs admitted to different groups are two concurrent Renovate processes against one repository, risking conflicting rebases and force-pushes. The anti-loop guard does nothing about this: it decides whether one run's job executes, never whether two already-admitted runs may run at once. That trades a cosmetic failure for a correctness problem.
Dropping the
pull_request: editedtrigger from callers. It removes the rebase checkbox, which is a mechanism people use, and it would mean editing every caller rather than fixing the cause here.What still holds
skipped, notcancelledorfailure.The other reusable workflows
Checked all of them. Only this one declared workflow-level concurrency. The others declare none.
Three caller workflows in this repository do declare it, and none has the identical defect: two already group per pull request or ref, and the release workflow groups by workflow name but its triggers cannot produce a burst across many pull requests, since one release pull request exists at a time. Left untouched and flagged rather than changed.
Verification
task lint:actionsandtask lint:yamlclean.task checkpasses in full, including the shell test suite.The workflow itself was not exercised. A reusable workflow cannot be triggered standalone, so the fix is proven only once a caller runs it. The first Renovate burst after this merges is the real test: the runs it would previously have cancelled should report
skipped.Blast radius
Every repository in the organisation consumes this workflow, so this changes all of them at once. That is the intent: the defect is in the shared file, and patching symptoms per repository would leave the cause in place.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.